Question Page Redesign – Page shell#4646
Conversation
* feat: implement responsive meta row for question metrics and relocated project chips * fix: sync meta row visibility breakpoints to prevent duplicated rendering and refine question header alignment layout * refactor: replace trophy icon dedup, manual mousedown listener with HeadlessUI Popover, and use translations generate script * fix: update Portuguese overflow label translation and fix vote score rendering condition
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughAdds a new QuestionPageShell with tabs, refactors sidebar/similar-questions, updates comments and charts, removes legacy layouts, adds i18n strings, switches Storybook to shells, and updates backend serialization to include user forecasts. ChangesQuestion Page Shell and integrations
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
|
🚀 Preview EnvironmentYour preview environment is ready!
Details
ℹ️ Preview Environment InfoIsolation:
Limitations:
Cleanup:
|
* feat(ui): build new TitleRow component with updated typography and variant-specific layout logic * fix: move question info on top in forecaster header
* feat: added new question action row with unified pill styling, dynamic primary logic, and add custom icons * refactor: consolidate pill styles into CVA + PillButton, replace hardcoded hex with design tokens, and fix Follow active state styling * refactor: remove action buttons from sidebar * fix: add dark mode variants to all pill styles * fix: remove dead imports and stale questionTitle prop from Sidebar
…/action rows (#4657) * feat: added new question action row with unified pill styling, dynamic primary logic, and add custom icons * refactor: consolidate pill styles into CVA + PillButton, replace hardcoded hex with design tokens, and fix Follow active state styling * fix: remove dead imports and stale questionTitle prop from Sidebar * feat: filter PostDropdownMenu for desktop and polish mobile consumer/forecaster meta and action rows * fix: show full action row from 768px, adjust Predict/Share on mobile, and gate dropdown Share/Follow/Embed below md
feat: add desktop tab bar with shared active-tab state
feat: add tab content adapters
* feat: change sidebar reflow for forecaster and consumer views * feat: redesign similar questions sidebar with new card layout and charts * feat: differentiate similar questions chart display between forecaster and consumer views * feat: add vertical bar chart for date/numeric group questions in similar questions consumer sidebar * fix: disable frontend cache for similar posts endpoint to ensure per-user vote state is returned correctly * fix: limit vertical bar chart to visible choices count to prevent overlapping labels * fix: replace VerticalBarConsumerCard with GroupForecastCard in similar questions sidebar and remove hideResolutionIcon prop * Similar questions endpoint: added user context to the API response --------- Co-authored-by: hlbmtc <hlib@metaculus.com>
* feat: integrate shell, widen column to 59rem, delete dead layouts * fix: CommunityDisclaimer placement and add shell z-index * feat: replace direct DOM scrolling with context-based comment navigation, improve cross-tab comment access, and refine consumer shell layout handling for different question types * feat: refine question page and comment feed styling, spacing, and layout * fix: stack comment date below author name on question page and align date/vote typography to design spec * feat: add My Scores tab for consumer, wire PostScoreData into forecaster shell with Resolution Criteria and Background Info, and fix duplicate comments on tab re-entry * refactor: gate My Scores tab on user scores, extract comment header condition into named vars, and deduplicate post score routing logic * fix: forward mobileSidebar to ConsumerShell, fix translated banner timer cleanup, update my-scores hash key, and widen Storybook story wrappers to 59rem * fix: restore preselectedGroupQuestionId in ConsumerShell for group and fan graph questions
1d400cf to
282a591
Compare
* fix: mobile consumer layout: center title, show prediction for all question types, fix comments tab blank below lg * feat: consumer view mobile - typography, spacing, tab bar sizing, horizontal scroll, and timeline in tab on mobile * feat: adjust title typography, vote count on mobile, and restore Forecast Timeline label across all views * feat: consumer/forecaster mobile - similar questions tab, hide sidebar author section on mobile, shared chip helpers with TrophyIcon, fix continuous graph clipping, and consumer title right padding * feat: consumer mobile - remove group prediction top margin on mobile and show compact bar chart for date groups instead of scatter * fix: format posts/views.py * fix: mobile polish pass, vote count size, comment text styling, section toggle arrow, and key factors link * fix: restrict fan graph/date group bottom margin to desktop only * fix: align x-axis tick labels with chart boundaries * fix: restructure consumer chart layout for date/fan graph groups and fix CommunityDisclaimer to only show for Community tournament type * refactor: migrate similar posts fetching to TanStack Query, co-located in SimilarQuestionsTab * feat: fall back to top-50 hot questions when no similar questions exist * fix: skip top-posts fallback fetch when similar questions already exist * fix: improve top-questions fallback with correct fetch params, loading state, and error handling * fix: sort tailwind classes to satisfy prettier lint rule
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
posts/views.py (1)
104-116:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
include_user_forecasts=Trueon the feed endpoint: verify necessity and performance cost
with_cpinposts_list_api_viewhas no default — when a client omits it the flag silently has no effect (see the serializer comment). Whenwith_cp=trueIS requested, every authenticated feed page now also executesprefetch_user_forecasts(current_user.id)across all paginated posts. If feed cards don't actually rendermy_forecastsdata, this is a free performance regression. Please confirm the feed-card UI consumes this data before keeping the flag here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@posts/views.py` around lines 104 - 116, The feed view is unconditionally passing include_user_forecasts=True to serialize_post_many which triggers prefetch_user_forecasts whenever with_cp is requested, causing a potential perf regression; verify whether the feed card UI actually renders my_forecasts and if not remove the flag, otherwise gate it so you only pass include_user_forecasts=True when with_cp is truthy and the user is authenticated and the UI requires forecasts (e.g., check request.user.is_authenticated and the with_cp flag or a dedicated query param before calling serialize_post_many); also consider adding a default for with_cp handling in posts_list_api_view or explicitly reading request.GET to avoid the silent no-op described.front_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/similar_question_prediction_chip.tsx (1)
91-98:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
return nullfallback in the forecaster branchThe consumer branch explicitly returns
nullfor unmatched posts (line 57), but the forecaster branch falls off the end if none ofisMultipleChoicePost,isQuestionPost, orisGroupOfQuestionsPostmatch. React will throw a runtime error if a component returnsundefined.🐛 Proposed fix
if (isGroupOfQuestionsPost(post)) { return ( <div className="w-full"> <GroupOfQuestionsTile post={post} showChart={false} /> </div> ); } + + return null; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front_end/src/app/`(main)/questions/[id]/components/sidebar/similar_questions/similar_question_prediction_chip.tsx around lines 91 - 98, The forecaster branch in SimilarQuestionPredictionChip can fall through and return undefined when none of isMultipleChoicePost, isQuestionPost, or isGroupOfQuestionsPost match; update the component to explicitly return null as a fallback (after the GroupOfQuestionsTile branch) so it mirrors the consumer branch's behavior and avoids React runtime errors.
🧹 Nitpick comments (8)
front_end/src/app/(main)/questions/components/forecaster_counter.tsx (1)
65-70: 💤 Low value
strongpassthrough silently stripsRichText's defaultstrongrenderer whenboldCountisfalse.The explicit
strongkey at line 65 overrides whatever handler...tags(spread fromRichText) injected for<strong>. WhenboldCountisfalse, rawchunksare returned unwrapped, discarding the default rendering contract fromRichText. IfRichText'sstrongtag ever applies its own styling (e.g.font-semibold), the non-bold path will silently drop it.If the intent is strictly "bold only when
boldCount=true, otherwise no emphasis at all", this is correct. Otherwise, consider falling back totags.strong:♻️ Proposed defensive passthrough
- strong: (chunks) => - boldCount ? ( - <span className="font-bold">{chunks}</span> - ) : ( - chunks - ), + strong: boldCount + ? (chunks) => <span className="font-bold">{chunks}</span> + : tags.strong,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front_end/src/app/`(main)/questions/components/forecaster_counter.tsx around lines 65 - 70, The current custom `strong` renderer in the forecaster_counter component overrides RichText's default handler and returns raw `chunks` when `boldCount` is false, dropping any default styling; update the `strong` passthrough so that when `boldCount` is true it wraps with your bold markup, otherwise it calls and returns the original `tags.strong` handler (or the default renderer) so RichText's intended styling/behavior is preserved; look for the `strong: (chunks) =>` entry near the `...tags` spread and change the false branch to delegate to `tags.strong` (or equivalent) instead of returning raw `chunks`.posts/views.py (1)
567-583:with_cp=Trueon the similar-posts endpoint conflicts with its own "don't overload Redis" commentThe endpoint previously had a lighter serialization call specifically to avoid Redis pressure. The new
with_cp=Truetriggersprefetch_questions_scores()for every similar-post lookup.group_cutoff=1helps, but the change is still a net increase. Consider whether CP data is strictly required here for the sidebar design, or whether a lighter approach (e.g., skippingwith_cp, or caching the response at the view level) would be sufficient.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@posts/views.py` around lines 567 - 583, The similar-posts view post_similar_posts_api_view currently calls serialize_post_many(..., with_cp=True) which forces prefetch_questions_scores and increases Redis load; change this to a lighter serialization by removing or setting with_cp=False (or add a flag to only include CP when explicitly requested) so serialize_post_many is called without triggering prefetch_questions_scores, or alternatively implement view-level caching around the serialized result in post_similar_posts_api_view to avoid repeated CP prefetches; update the call site (serialize_post_many in post_similar_posts_api_view) and any tests that assume CP fields are present.front_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs/comments.tsx (1)
12-16: ⚖️ Poor tradeoffConsider adding a flat/no-container-styles prop to
CommentFeedinstead of the!importantselector chain.The wrapper div's
[&>section]:!*overrides are fragile — they silently break ifCommentFeed's internal element ever changes from<section>to something else. AnoContainerStyles(or similar) prop onCommentFeedwould be more resilient and expressive.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front_end/src/app/`(main)/questions/[id]/components/question_page_shell/tabs/comments.tsx around lines 12 - 16, The wrapper uses fragile tailwind child selector overrides on <section> to remove container styles; instead add a boolean prop (e.g., noContainerStyles or flat) to the CommentFeed component and have CommentFeed apply the containerless classes internally when that prop is true. Update the CommentsTab component to pass noContainerStyles (or flat) to CommentFeed and remove the `[&>section]:!*` overrides so the styling control is explicit and resilient to internal markup changes.front_end/src/app/(main)/questions/[id]/components/question_page_shell/meta_row.tsx (1)
49-54: 💤 Low valueInitial render shows default chip count (2) regardless of container width.
When
width === 0(before the resize observer fires on first paint),maxVisibleChipsevaluates to2andcompactCounterstofalse. This is fine as a safe default, but it will cause a visible layout shift on narrow containers. Consider initializinguseContainerSizeor the width thresholds more conservatively (e.g., defaultmaxVisibleChipsto1) if the layout shift is noticeable in practice.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front_end/src/app/`(main)/questions/[id]/components/question_page_shell/meta_row.tsx around lines 49 - 54, The initial render uses width === 0 which causes maxVisibleChips to default to 2 and compactCounters to false, creating a layout shift; update the logic in the component around compactCounters and maxVisibleChips (the variables computed from width, and where useContainerSize provides width) to treat width <= 0 as an unknown/initial state and return conservative defaults (e.g., set maxVisibleChips = 1 and compactCounters = true when width <= 0) so the first paint matches narrow-container behavior until the resize observer fires.front_end/src/stories/question_page/date_question/date_question.stories.tsx (1)
76-76: 💤 Low value
buildKeycan return an empty string.When no transform rules apply,
appliedKeysis empty and this returns""as the key, which can be problematic for React reconciliation. Other migrated stories in this PR (e.g.,binary_group_timeline.stories.tsx) use a"default"fallback. Consider aligning for consistency:Proposed alignment
- buildKey: (_, appliedKeys) => `${appliedKeys.join("-")}`, + buildKey: (_, appliedKeys) => + appliedKeys.length > 0 ? appliedKeys.join("-") : "default",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front_end/src/stories/question_page/date_question/date_question.stories.tsx` at line 76, The buildKey function currently returns an empty string when no transform rules apply because appliedKeys is empty; update the buildKey implementation (the buildKey property in date_question.stories.tsx) to return a non-empty fallback (e.g., "default") when appliedKeys.length === 0 so React reconciliation is stable and consistent with other stories like binary_group_timeline.stories.tsx.front_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/similar_question_card.tsx (1)
109-136: 💤 Low valueForecaster footer chip styling looks slightly inconsistent.
Three sibling chips share the same row but use different backgrounds/padding:
- Comment chip (Line 112):
rounded-xs bg-gray-200 px-1.5- Status icon container (Line 119):
rounded-xs bg-gray-200 px-1(narrower padding)- Forecasters chip (Line 128): no background, no rounding (
text-xs leading-4 text-gray-700only)If this is the intended design (e.g., status badge gets tighter padding, forecasters count is unboxed), feel free to ignore. Otherwise consider unifying them, since visually adjacent counters typically benefit from a single chip style.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front_end/src/app/`(main)/questions/[id]/components/sidebar/similar_questions/similar_question_card.tsx around lines 109 - 136, The three adjacent footer chips in SimilarQuestionCard (rendered when isForecaster) use inconsistent classes: the comment span, the PostStatusIcon container, and the forecasters span; unify their appearance by applying a consistent chip class set (e.g., "flex h-6 items-center gap-1 rounded-xs bg-gray-200 px-1.5 text-xs leading-4 text-gray-700 dark:bg-gray-200-dark dark:text-gray-700-dark") to the comment span, the wrapper div around PostStatusIcon, and the forecasters span so PostVoter, PostStatusIcon wrapper, and the forecasters count (forecastersFormatted) all share the same background, padding, rounding and text styles.front_end/src/app/(main)/questions/[id]/components/question_page_shell/title_row.tsx (1)
42-55: 💤 Low valueRemove ineffective
order-*utilities from single-child flex container.The div at line 43 is the only child within its flex parent (line 42), so
order-1 lg:order-0are dead code—CSSorderonly reorders flex items relative to siblings. Additionally,lg:order-0is not part of Tailwind's defaultorderscale (which supportsorder-1–order-12,order-first,order-last,order-none) and the project config does not extend this scale, solg:order-0silently produces no CSS.If reordering the title row relative to the desktop CP status block at line 56 was intended, these utilities should be applied to siblings at the parent level (line 38), not nested inside line 42.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front_end/src/app/`(main)/questions/[id]/components/question_page_shell/title_row.tsx around lines 42 - 55, The order utilities on the inner container (the div with class "lg:order-0 order-1 flex items-center") are ineffective because its parent (the div with class "flex flex-1 flex-col") has only that single child; remove the `order-1` and `lg:order-0` classes from that inner div. If you intended to reorder the title vs. the desktop CP status (components QuestionTitle and QuestionHeaderCPStatus), move the order classes to the relevant sibling containers at the parent level so the `order-*` utilities affect sibling ordering instead of applying them on this single-child element.front_end/src/app/(main)/questions/[id]/components/question_page_shell/index.tsx (1)
77-84: ⚡ Quick winDuplicate
CommunityDisclaimerblock — consider extracting to a helperThe exact same
CommunityDisclaimersnippet appears in bothForecasterShell(lines 77-84) andConsumerShell(lines 175-182). A small helper or shared fragment would remove the duplication.♻️ Proposed refactor
+const CommunityBanner: FC<{ post: PostWithForecasts }> = ({ post }) => + post.projects?.default_project?.type === TournamentType.Community ? ( + <CommunityDisclaimer + project={post.projects.default_project} + variant="standalone" + className="block sm:hidden" + /> + ) : null; // ForecasterShell - {postData.projects?.default_project?.type === - TournamentType.Community && ( - <CommunityDisclaimer - project={postData.projects.default_project} - variant="standalone" - className="block sm:hidden" - /> - )} + <CommunityBanner post={postData} /> // ConsumerShell - {postData.projects?.default_project?.type === - TournamentType.Community && ( - <CommunityDisclaimer - project={postData.projects.default_project} - variant="standalone" - className="block sm:hidden" - /> - )} + <CommunityBanner post={postData} />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@front_end/src/app/`(main)/questions/[id]/components/question_page_shell/index.tsx around lines 77 - 84, Duplicate JSX rendering of CommunityDisclaimer exists in ForecasterShell and ConsumerShell; extract it into a small helper/component (e.g., renderCommunityDisclaimer or CommunityDisclaimerFragment) and replace both inline blocks with a single call. The helper should accept the same props used now (project: postData.projects.default_project, variant: "standalone", className: "block sm:hidden") and perform the conditional check postData.projects?.default_project?.type === TournamentType.Community before returning <CommunityDisclaimer /> so both ForecasterShell and ConsumerShell simply invoke the helper to remove duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@front_end/messages/cs.json`:
- Line 1823: The Czech value for the translation key questionLinks reads
awkwardly; update the JSON entry for "questionLinks" to use a natural
prepositional form—replace "Odkazy otázky" with "Odkazy na otázky" so the UI
reads correctly. Ensure you update the string value for the questionLinks key in
front_end/messages/cs.json accordingly and keep JSON quoting/commas intact.
In `@front_end/src/app/`(main)/questions/[id]/components/post_score_data/utils.ts:
- Around line 23-28: shouldQuestionShowUserScores currently returns true
whenever question.my_forecasts?.score_data exists, but it lacks the
unsuccessful-resolution guard present in shouldQuestionShowScores; update
shouldQuestionShowUserScores to also return false when
isUnsuccessfullyResolved(question.resolution) is true (i.e., add a check like
!isUnsuccessfullyResolved(question.resolution) combined with the existing
user_scores check), referencing the same question.my_forecasts?.score_data and
isUnsuccessfullyResolved symbol to ensure the "My Scores" tab stays hidden for
annulled/ambiguous resolutions.
In
`@front_end/src/app/`(main)/questions/[id]/components/question_page_shell/tab_bar.tsx:
- Around line 95-99: The UI shows a different highlighted tab than the panel
because the local `active` fallback in tab_bar.tsx is not syncing back to the
layout context when the current `activeTab` is removed; add a useEffect in the
TabBar component that watches `active` and `activeTab`/`tabs` and, when
`activeTab` is not present in `tabs`, calls the context updater (e.g.,
`setActiveTab`) to set the context to the computed fallback `active` so
`renderActivePanel` in tabs.tsx reads the same value the <Tabs> control
displays.
In `@front_end/src/components/charts/primitives/x_tick_label.tsx`:
- Around line 26-29: The right-boundary check is asymmetric: when withCursor is
false the code uses a fixed 12px threshold causing wide labels
(estimatedTextWidth > 12) to overflow; in the conditional around x/chartWidth
replace the ternary so both branches use a half-width-aware guard (e.g. x >
chartWidth - estimatedTextWidth) or, if you deliberately want a positional 12px
guard when withCursor is false, add a clarifying comment explaining why the
thresholds differ; update the expression that sets textAnchor (the block
referencing withCursor, x, chartWidth, estimatedTextWidth, and textAnchor)
accordingly.
In `@front_end/src/components/comment_feed/comment.tsx`:
- Around line 1081-1083: Replace the hardcoded aria-label on the dropdown
trigger Button with a localized string: call useTranslations() at the top of the
component (e.g., const t = useTranslations()) and change aria-label="menu" to
aria-label={t("moreOptions") || t("menu")}, ensuring the key exists in your i18n
files; update the component that renders the Button (the Comment component / the
dropdown trigger Button) and add the necessary import for useTranslations to the
file.
In `@front_end/src/components/comment_feed/index.tsx`:
- Around line 348-351: The sort/count row is rendering regardless of compact
mode; wrap or gate the sort-and-count UI using the same compactVersion check as
showCommentHeader (i.e., ensure the sort/count block is only rendered when
!compactVersion). Locate the sort/count JSX near the comment header rendering
(references: showCommentHeader, compactVersion) and move it inside the same
conditional or add a condition like !compactVersion so callers such as
ResponsiveCommentFeed receive a true compact view with those elements
suppressed.
In `@front_end/src/components/post_card/basic_post_card/comment_status.tsx`:
- Line 57: The className string contains a merged token "border-nonepx-2" which
should be split and de-duplicated; update the string so it contains "border-none
px-2" (remove the duplicate extra "border-none" if present elsewhere in that
same string) so Tailwind correctly applies the px-2 horizontal padding; locate
and fix the literal class string in comment_status.tsx where "border-nonepx-2
h-6..." appears.
In `@front_end/src/components/post_card/basic_post_card/post_voter.tsx`:
- Around line 80-88: The issue is the hardcoded "md:text-sm" in the static class
string overriding the compact branch at desktop; update the class logic in
post_voter.tsx so responsive sizing can be controlled by the compact prop:
remove the fixed "md:text-sm" and use a non-responsive "text-sm" as the default,
then let the compact conditional (compact ? "text-xs" : "text-sm") determine
sizing (or, alternatively, move "md:text-sm" into the non-compact branch).
Adjust the cn(...) call around the vote score span accordingly so compact=true
wins at md+.
In `@posts/serializers.py`:
- Around line 506-507: The prefetch of user forecasts is currently nested under
the with_cp conditional so include_user_forecasts=True can silently do nothing;
either document this dependency in the function signature or enforce it — e.g.,
add an assertion that include_user_forecasts implies with_cp, or move the call
to qs.prefetch_user_forecasts(current_user.id) out of the with_cp block so
include_user_forecasts takes effect independently; update the relevant function
signature/docstring and callers (e.g., posts_list_api_view) accordingly.
---
Outside diff comments:
In
`@front_end/src/app/`(main)/questions/[id]/components/sidebar/similar_questions/similar_question_prediction_chip.tsx:
- Around line 91-98: The forecaster branch in SimilarQuestionPredictionChip can
fall through and return undefined when none of isMultipleChoicePost,
isQuestionPost, or isGroupOfQuestionsPost match; update the component to
explicitly return null as a fallback (after the GroupOfQuestionsTile branch) so
it mirrors the consumer branch's behavior and avoids React runtime errors.
In `@posts/views.py`:
- Around line 104-116: The feed view is unconditionally passing
include_user_forecasts=True to serialize_post_many which triggers
prefetch_user_forecasts whenever with_cp is requested, causing a potential perf
regression; verify whether the feed card UI actually renders my_forecasts and if
not remove the flag, otherwise gate it so you only pass
include_user_forecasts=True when with_cp is truthy and the user is authenticated
and the UI requires forecasts (e.g., check request.user.is_authenticated and the
with_cp flag or a dedicated query param before calling serialize_post_many);
also consider adding a default for with_cp handling in posts_list_api_view or
explicitly reading request.GET to avoid the silent no-op described.
---
Nitpick comments:
In
`@front_end/src/app/`(main)/questions/[id]/components/question_page_shell/index.tsx:
- Around line 77-84: Duplicate JSX rendering of CommunityDisclaimer exists in
ForecasterShell and ConsumerShell; extract it into a small helper/component
(e.g., renderCommunityDisclaimer or CommunityDisclaimerFragment) and replace
both inline blocks with a single call. The helper should accept the same props
used now (project: postData.projects.default_project, variant: "standalone",
className: "block sm:hidden") and perform the conditional check
postData.projects?.default_project?.type === TournamentType.Community before
returning <CommunityDisclaimer /> so both ForecasterShell and ConsumerShell
simply invoke the helper to remove duplication.
In
`@front_end/src/app/`(main)/questions/[id]/components/question_page_shell/meta_row.tsx:
- Around line 49-54: The initial render uses width === 0 which causes
maxVisibleChips to default to 2 and compactCounters to false, creating a layout
shift; update the logic in the component around compactCounters and
maxVisibleChips (the variables computed from width, and where useContainerSize
provides width) to treat width <= 0 as an unknown/initial state and return
conservative defaults (e.g., set maxVisibleChips = 1 and compactCounters = true
when width <= 0) so the first paint matches narrow-container behavior until the
resize observer fires.
In
`@front_end/src/app/`(main)/questions/[id]/components/question_page_shell/tabs/comments.tsx:
- Around line 12-16: The wrapper uses fragile tailwind child selector overrides
on <section> to remove container styles; instead add a boolean prop (e.g.,
noContainerStyles or flat) to the CommentFeed component and have CommentFeed
apply the containerless classes internally when that prop is true. Update the
CommentsTab component to pass noContainerStyles (or flat) to CommentFeed and
remove the `[&>section]:!*` overrides so the styling control is explicit and
resilient to internal markup changes.
In
`@front_end/src/app/`(main)/questions/[id]/components/question_page_shell/title_row.tsx:
- Around line 42-55: The order utilities on the inner container (the div with
class "lg:order-0 order-1 flex items-center") are ineffective because its parent
(the div with class "flex flex-1 flex-col") has only that single child; remove
the `order-1` and `lg:order-0` classes from that inner div. If you intended to
reorder the title vs. the desktop CP status (components QuestionTitle and
QuestionHeaderCPStatus), move the order classes to the relevant sibling
containers at the parent level so the `order-*` utilities affect sibling
ordering instead of applying them on this single-child element.
In
`@front_end/src/app/`(main)/questions/[id]/components/sidebar/similar_questions/similar_question_card.tsx:
- Around line 109-136: The three adjacent footer chips in SimilarQuestionCard
(rendered when isForecaster) use inconsistent classes: the comment span, the
PostStatusIcon container, and the forecasters span; unify their appearance by
applying a consistent chip class set (e.g., "flex h-6 items-center gap-1
rounded-xs bg-gray-200 px-1.5 text-xs leading-4 text-gray-700
dark:bg-gray-200-dark dark:text-gray-700-dark") to the comment span, the wrapper
div around PostStatusIcon, and the forecasters span so PostVoter, PostStatusIcon
wrapper, and the forecasters count (forecastersFormatted) all share the same
background, padding, rounding and text styles.
In `@front_end/src/app/`(main)/questions/components/forecaster_counter.tsx:
- Around line 65-70: The current custom `strong` renderer in the
forecaster_counter component overrides RichText's default handler and returns
raw `chunks` when `boldCount` is false, dropping any default styling; update the
`strong` passthrough so that when `boldCount` is true it wraps with your bold
markup, otherwise it calls and returns the original `tags.strong` handler (or
the default renderer) so RichText's intended styling/behavior is preserved; look
for the `strong: (chunks) =>` entry near the `...tags` spread and change the
false branch to delegate to `tags.strong` (or equivalent) instead of returning
raw `chunks`.
In `@front_end/src/stories/question_page/date_question/date_question.stories.tsx`:
- Line 76: The buildKey function currently returns an empty string when no
transform rules apply because appliedKeys is empty; update the buildKey
implementation (the buildKey property in date_question.stories.tsx) to return a
non-empty fallback (e.g., "default") when appliedKeys.length === 0 so React
reconciliation is stable and consistent with other stories like
binary_group_timeline.stories.tsx.
In `@posts/views.py`:
- Around line 567-583: The similar-posts view post_similar_posts_api_view
currently calls serialize_post_many(..., with_cp=True) which forces
prefetch_questions_scores and increases Redis load; change this to a lighter
serialization by removing or setting with_cp=False (or add a flag to only
include CP when explicitly requested) so serialize_post_many is called without
triggering prefetch_questions_scores, or alternatively implement view-level
caching around the serialized result in post_similar_posts_api_view to avoid
repeated CP prefetches; update the call site (serialize_post_many in
post_similar_posts_api_view) and any tests that assume CP fields are present.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ce74cd31-71e0-45ff-b0de-a081f967046e
📒 Files selected for processing (89)
front_end/messages/cs.jsonfront_end/messages/en.jsonfront_end/messages/es.jsonfront_end/messages/pt.jsonfront_end/messages/zh-TW.jsonfront_end/messages/zh.jsonfront_end/src/app/(main)/components/comments_feed_provider.tsxfront_end/src/app/(main)/questions/[id]/[[...slug]]/page_component.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/key_factor_header.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/item_view/more_panel.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/key_factor_detail_overlay.tsxfront_end/src/app/(main)/questions/[id]/components/key_factors/key_factors_question_consumer_section.tsxfront_end/src/app/(main)/questions/[id]/components/post_score_data/single_question_score_data.tsxfront_end/src/app/(main)/questions/[id]/components/post_score_data/utils.tsfront_end/src/app/(main)/questions/[id]/components/question_layout/consumer_question_layout/consumer_tabs.tsxfront_end/src/app/(main)/questions/[id]/components/question_layout/consumer_question_layout/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_layout/consumer_question_layout/responsive_comment_feed.tsxfront_end/src/app/(main)/questions/[id]/components/question_layout/forecaster_question_layout/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_layout/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_layout/question_info.tsxfront_end/src/app/(main)/questions/[id]/components/question_layout/question_layout_context.tsxfront_end/src/app/(main)/questions/[id]/components/question_layout/question_section.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/meta_row.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/project_chip_helpers.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/tab_bar.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs/comments.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs/key_factors.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs/my_scores.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs/private_notes.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs/question_info.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs/question_links.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs/similar_questions.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/tabs/timeline.tsxfront_end/src/app/(main)/questions/[id]/components/question_page_shell/title_row.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/action_row.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/action_buttons/question_predict_button.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/prediction/group_of_questions_prediction/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/prediction/single_question_prediction/binary_question_prediction.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/prediction/single_question_prediction/continuous_question_prediction.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/timeline/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/forecaster_question_view/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/forecaster_question_view/question_header/index.tsxfront_end/src/app/(main)/questions/[id]/components/question_view/index.tsxfront_end/src/app/(main)/questions/[id]/components/sidebar/index.tsxfront_end/src/app/(main)/questions/[id]/components/sidebar/sidebar_container.tsxfront_end/src/app/(main)/questions/[id]/components/sidebar/sidebar_question_info.tsxfront_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/index.tsxfront_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/similar_question_card.tsxfront_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/similar_question_prediction_chip.tsxfront_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/similar_questions_drawer.tsxfront_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/similar_questions_list.tsxfront_end/src/app/(main)/questions/components/coherence_links/coherence_links.tsxfront_end/src/app/(main)/questions/components/forecaster_counter.tsxfront_end/src/components/charts/group_chart.tsxfront_end/src/components/charts/primitives/x_tick_label.tsxfront_end/src/components/comment_feed/comment.tsxfront_end/src/components/comment_feed/comment_card.tsxfront_end/src/components/comment_feed/comment_date.tsxfront_end/src/components/comment_feed/comment_voter.tsxfront_end/src/components/comment_feed/comment_wrapper.tsxfront_end/src/components/comment_feed/index.tsxfront_end/src/components/consumer_post_card/group_forecast_card/index.tsxfront_end/src/components/detailed_question_card/detailed_question_card/index.tsxfront_end/src/components/icons/share.tsxfront_end/src/components/icons/trophy.tsxfront_end/src/components/post_actions/post_dropdown_menu.tsxfront_end/src/components/post_card/basic_post_card/comment_status.tsxfront_end/src/components/post_card/basic_post_card/post_voter.tsxfront_end/src/components/post_card/question_tile/question_continuous_tile.tsxfront_end/src/components/post_status/index.tsxfront_end/src/components/question/private_note.tsxfront_end/src/components/ui/section_toggle.tsxfront_end/src/components/voter.tsxfront_end/src/services/api/posts/posts.shared.tsfront_end/src/stories/question_page/binary_group/binary_group_timeline.stories.tsxfront_end/src/stories/question_page/binary_group/fan_chart.stories.tsxfront_end/src/stories/question_page/binary_question/binary_question.stories.tsxfront_end/src/stories/question_page/continuous_question/continuous_question.stories.tsxfront_end/src/stories/question_page/date_group/date_group_timeline.stories.tsxfront_end/src/stories/question_page/date_group/fan_chart.stories.tsxfront_end/src/stories/question_page/date_question/date_question.stories.tsxfront_end/src/stories/question_page/mc_question/mc_question.stories.tsxfront_end/src/stories/question_page/numeric_group/numeric_fan_chart.stories.tsxfront_end/src/stories/question_page/numeric_group/numeric_group_timeline.stories.tsxposts/serializers.pyposts/views.py
💤 Files with no reviewable changes (11)
- front_end/src/app/(main)/questions/[id]/components/question_layout/consumer_question_layout/index.tsx
- front_end/src/app/(main)/questions/[id]/components/sidebar/similar_questions/similar_questions_drawer.tsx
- front_end/src/app/(main)/questions/[id]/components/question_layout/forecaster_question_layout/index.tsx
- front_end/src/app/(main)/questions/[id]/components/question_view/forecaster_question_view/question_header/index.tsx
- front_end/src/app/(main)/questions/[id]/components/question_layout/question_section.tsx
- front_end/src/app/(main)/questions/[id]/components/question_view/index.tsx
- front_end/src/app/(main)/questions/[id]/components/question_layout/consumer_question_layout/consumer_tabs.tsx
- front_end/src/app/(main)/questions/[id]/components/question_view/forecaster_question_view/index.tsx
- front_end/src/app/(main)/questions/[id]/components/question_view/consumer_question_view/index.tsx
- front_end/src/app/(main)/questions/[id]/components/question_layout/question_info.tsx
- front_end/src/app/(main)/questions/[id]/components/question_layout/index.tsx
| if current_user and include_user_forecasts: | ||
| qs = qs.prefetch_user_forecasts(current_user.id) |
There was a problem hiding this comment.
include_user_forecasts silently has no effect when with_cp=False
The prefetch is nested inside the if with_cp: block, so callers that pass include_user_forecasts=True without also ensuring with_cp=True (e.g., posts_list_api_view where with_cp comes from an optional query param and defaults to None) will get no user-forecast data without any indication. At minimum, document the dependency in the function signature; a stricter option is an assertion.
🛡️ Suggested defensive guard
+ if include_user_forecasts and not with_cp:
+ raise ValueError("include_user_forecasts requires with_cp=True")
if with_cp:
qs = qs.prefetch_questions_scores()
if current_user and include_user_forecasts:
qs = qs.prefetch_user_forecasts(current_user.id)Or, as a softer option, move the prefetch outside the with_cp guard (if user forecasts are needed independently of CP data).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@posts/serializers.py` around lines 506 - 507, The prefetch of user forecasts
is currently nested under the with_cp conditional so include_user_forecasts=True
can silently do nothing; either document this dependency in the function
signature or enforce it — e.g., add an assertion that include_user_forecasts
implies with_cp, or move the call to qs.prefetch_user_forecasts(current_user.id)
out of the with_cp block so include_user_forecasts takes effect independently;
update the relevant function signature/docstring and callers (e.g.,
posts_list_api_view) accordingly.
…on resize, chart label right-boundary overflow, compact UI regressions, and Tailwind class typo in comment status
Closes #4638
This PR delivers the first iteration of the Question Page redesign by introducing a new unified page shell architecture and rebuilding the main page structure for both Consumer and Forecaster views across desktop and mobile.
Implemented in Iteration 1
QuestionPageShellarchitecture that replaces legacy question layout components and centralizes page rendering logicUI / Layout updates
Sidebar & content changes
Functional improvements & fixes
Cleanup
Summary by CodeRabbit
New Features
UI/UX Improvements
Internationalization